Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because a core requirement of the task is not yet met: Array.prototype.sort is not actually wired to use your custom sort2 implementation, so arr.sort() still calls the native engine sort. The task explicitly requires that the standard sort method be implemented via the predefined sort2, which means that after defining sort2 (e.g. in src/arrayMethodSort.js, around the lines where you attach sort2 to [].__proto__), you must assign something like:
Array.prototype.sort = Array.prototype.sort2;or delegate sort to sort2 inside applyCustomSort. This missing linkage is a functional blocker, because tests or consumers that call arr.sort() will not see your custom behavior at all. On the positive side, your insertion-sort logic inside sort2 looks correct, handles default comparators, and returns this, which matches the native API shape. Once you connect Array.prototype.sort to sort2 and ensure that this happens when the module is used (e.g. inside applyCustomSort or at module load, depending on the task instructions), the solution should satisfy the main requirements. As a minor, non-blocking suggestion, you could consider using Object.getPrototypeOf([]) instead of __proto__ for style, but fixing the sort linkage is the only change strictly needed to pass.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| this[j + 1] = key; | ||
| } | ||
|
|
||
| return this; |
There was a problem hiding this comment.
The task requires Array.prototype.sort to be implemented via the predefined sort2. You define sort2 but never assign it to Array.prototype.sort. After the sort2 definition add an assignment such as Array.prototype.sort = Array.prototype.sort2; so normal arr.sort() calls use your implementation.
| const cmp = | ||
| typeof compareFunction === 'function' ? compareFunction : defaultCompare; | ||
|
|
||
| for (let i = 1; i < this.length; i++) { |
There was a problem hiding this comment.
The loop implements an insertion-sort-style algorithm (acceptable for this exercise). Note: for large arrays this is O(n²); consider mentioning or using a more efficient algorithm only if performance is a concern (not required for this task).
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved because it correctly implements a custom Array.prototype.sort using [].__proto__.sort2 and then assigns Array.prototype.sort = Array.prototype.sort2, which matches the task requirements. The insertion-style algorithm you wrote correctly handles the optional compare function, shifts elements as needed, and returns this, so the core behavior of sort is satisfied. There are no critical or blocking issues with functionality or compliance with the checklist items.
For potential improvements (not required for this task), you might add a short comment above applyCustomSort explaining that it patches Array.prototype.sort globally so future readers understand the side effects. You could also consider adding tests for edge cases like sparse arrays or arrays with holes to better document and verify behavior compared to the native sort. Finally, be aware that your insertion-sort approach is O(n²); this is fine here, but for very large arrays a more efficient algorithm (like quicksort or mergesort) would scale better. Overall, solid work implementing the custom sort and wiring it into the prototype correctly.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.